Skip to content

Opt-in accessors via renderer listen_to kwarg - #483

Open
cpsievert wants to merge 5 commits into
mainfrom
listen-to-opt-in
Open

cpsievert wants to merge 5 commits into
mainfrom
listen-to-opt-in

Conversation

@cpsievert

Copy link
Copy Markdown
Collaborator

Split out of #480; infrastructure for the plotly PR stacked on top of this one. AccessorSpec.opt_in marks accessors whose payloads are unbounded or stream continuously; the renderer skips attaching them at render time, and reading one raises an error pointing at listen_to, unless declared (@render_*(listen_to=["selected"])). listen_to is additive to the always-on accessors, and unknown names raise at decoration time. No accessor is opt-in yet, so this changes no user-visible behavior on its own.

@cpsievert

Copy link
Copy Markdown
Collaborator Author

@copilot please re-review

Copilot AI commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

@copilot please re-review

Re-review finding (P2): the new listen_to kwarg in shinywidgets/_render_widget_base.py:114 isn't accepted/forwarded by the explicit constructors for render_embedding_atlas, render_mosaic, or render_quak. For example, render_quak(listen_to=["sql"]) raises TypeError even though sql is a valid always-on accessor. render_mols2grid instead captures it in **kwargs and routes it to widget options, bypassing accessor validation. These constructors should explicitly accept and forward listen_to.

No files changed. Targeted tests could not run because this environment lacks both uv and pytest.

render_embedding_atlas, render_mosaic_widget, and render_quak didn't
accept the new listen_to kwarg (TypeError), and render_mols2grid
swallowed it in **kwargs, routing it to widget options instead of the
accessor validation. Each now takes listen_to explicitly and forwards
it to render_widget_base.
@cpsievert

Copy link
Copy Markdown
Collaborator Author

@copilot please re-review

Copilot AI commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

@cpsievert Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: a10405dd-a165-4f32-b62c-677d67ba78e7

Sorry for the inconvenience!

Copilot AI commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

@copilot please re-review

Re-reviewed all four commits. The constructor forwarding issue is resolved in c668733, with regression tests; no new actionable findings.

CI for c668733 passed all nine jobs. The Python 3.11 log confirms 1,154 tests passed (3 skipped, 1 expected failure). Local tests remain unavailable because uv and pytest aren't installed. No files changed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants